Prevent NPE in HttpClientDecorator.isAgentRequest for a null request - #12376
Prevent NPE in HttpClientDecorator.isAgentRequest for a null request#12376dougqh wants to merge 7 commits into
Conversation
ByteBuddy advice runs before the instrumented method's own argument validation, so a caller passing a null request (e.g. httpClient.sendAsync(null, ...)) NPEs inside isAgentRequest -> getRequestHeader before the JDK's own null-check gets a chance to fire, producing a spurious "Failed to handle exception in instrumentation" telemetry entry instead of the real NPE the JDK would have thrown. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dougqh
added
tag: ai generated
![datadog-prod-us1-4[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The null guard stops advice from reading headers from a null request. It preserves the instrumented method's own argument validation.
🤖 Datadog Autotest · Commit 49b13a5 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for 310723a: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
The merge request has been interrupted because the build 3148881699859800137 took longer than expected. The current limit for the base branch 'master' is 120 minutes. |
3 participants
What Does This Do
Guards
HttpClientDecorator.isAgentRequest(REQUEST)against anullrequest, returningfalseinstead of throwing anNPEfrom insidegetRequestHeader. Adds a JUnit 5 regression test.Motivation
ByteBuddy method advice runs before the instrumented method's own argument validation. So when a caller passes a null
HttpRequest(e.g.httpClient.sendAsync(null, bodyHandler)), ourSendAsyncAdvice/SendAdvicefor thejava.net.httpinstrumentation dereferences it first viaisAgentRequest(request)→getRequestHeader(request, ...)and NPEs — before the JDK's own null-check on that same argument ever gets a chance to fire its own, correctly-attributed NPE.The advice's
suppress = Throwable.classkeeps this from breaking the caller (the real method still runs afterward and throws its own NPE as expected), but the swallowed exception is logged/recorded as an instrumentation failure ("Failed to handle exception in instrumentation for ... - SendAsyncAdvice"), which is what surfaced this NPE misattributed as an instrumentation bug in Datadog error tracking (issue83642586-aab1-11f0-a63b-da7ad0900002).Fixing in the shared
HttpClientDecoratorbase class rather than only inJavaNetClientDecoratorso every HTTP client integration built on it is protected the same way.Additional Notes
techdebtandperf-reviewskills were run over this branch: no findings from either —isAgentRequestis on the per-request hot path, but the added guard is a single cheap null check with no allocation.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueJira ticket: N/A